Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/api/src/app/deployment/services/deployment-pr-linking.service.ts`:
- Around line 167-213: The DataIntegrityException message in
updatePullRequestDeploymentTracking contains a typo ("Depoloyed"); update the
thrown DataIntegrityException call to correct the text to "Deployed Pull Request
is not merged" (keep the surrounding context and the extra: { pullRequest }
payload untouched) so the error message reads correctly when
DataIntegrityException is raised.
- Around line 200-212: The update call to
getPrisma(...).pullRequestTracking.update assumes a tracking row exists and can
raise Prisma P2025 when pullRequest.tracking is null; change this to either (a)
guard before calling update and create the tracking row if missing, or (b)
replace the update with pullRequestTracking.upsert using pullRequest.id and
workspaceId as the unique key so it will create or update atomically; ensure
this prevents unhandled errors inside the Promise.all batch that processes
deployments.

In `@apps/api/src/app/deployment/services/deployment-pr-linking.types.ts`:
- Around line 38-43: The UpdatePullRequestDeploymentTrackingArgs type allows
pullRequest.tracking to be null, but updatePullRequestDeploymentTracking
currently calls prisma.pullRequestTracking.update() unguarded; change the
service to either (a) perform an early null check/return when
pullRequest.tracking is null before calling prisma.pullRequestTracking.update(),
or (b) replace the update call with prisma.pullRequestTracking.upsert(...) using
pullRequest.tracking?.id (or pullRequest.id) as the key and providing both
create and update payloads so a missing tracking record is created; update the
service code around prisma.pullRequestTracking.update() accordingly to ensure no
runtime throw when tracking is null.

In
`@apps/api/src/app/deployment/workers/deployment-triggered-by-pull-request-merge.worker.ts`:
- Around line 34-46: The pullRequest returned from findPullRequestById is
inferred with an optional include so TypeScript can't guarantee the tracking
field for the downstream call to createDeploymentFromPullRequestMerge; add a
local type assertion when passing pullRequest (assert as PullRequest & {
tracking: PullRequestTracking | null }) so the callsite satisfies the expected
shape, or alternatively change findPullRequestById to a generic overload that
narrows its return type based on the include parameter; update the call that
supplies pullRequest to createDeploymentFromPullRequestMerge to use the
asserted/narrowed type and ensure imports for PullRequest and
PullRequestTracking are present.

In `@apps/api/src/app/github/services/github-pull-request.service.ts`:
- Around line 195-211: fetchPullRequestMergeCommitSha currently calls
octokit.rest.pulls.get without error handling and can throw; wrap the REST call
in a try/catch inside fetchPullRequestMergeCommitSha, returning undefined on any
failure (matching getFirstCommit's behavior) and optionally log the error (e.g.,
console.error or the module logger) with context including installationId and
gitPrData.number; ensure the function still returns
response.data.merge_commit_sha || undefined on success.

In `@apps/api/src/lib/logger.ts`:
- Around line 29-35: The code currently mutates the caller's object because
const cleanObj = obj || {} only copies the reference; change cleanObj to be a
shallow copy (e.g., const cleanObj = obj ? { ...obj } : {}) so subsequent
cleanObj[key] = pick(...) does not modify obj, and ensure the sanitized object
(cleanObj) is passed to Sentry.addBreadcrumb (instead of obj); update usages
around cleanObj, loggableFields and pick to operate on the new shallow copy so
the original obj remains unchanged.
- Line 24: The loggable fields array for pullRequest is wrong — replace the
current array ["id", "name", "description", "createdAt", "updatedAt"] with the
correct PullRequest fields such as ["id", "title", "createdAt", "updatedAt"] (or
add other valid fields like "number" or "state" if desired) so the logger picks
up the PR title and valid attributes; update the pullRequest array definition in
logger.ts accordingly (refer to the pullRequest variable/constant where the
array is declared).
- Around line 31-34: The TypeScript error comes from indexing cleanObj (typed as
object) and loggableFields with a general string; update the usage in the loop
inside apps/api/src/lib/logger.ts (symbols: cleanObj, loggableFields, pick) so
cleanObj is treated as Record<string, unknown> (either change the parameter type
obj?: object to obj?: Record<string, unknown> or cast cleanObj to that type
before indexing) and guard the access to loggableFields with a proper key check
(use `if (key in loggableFields)` and index as `loggableFields[key as keyof
typeof loggableFields]`) before calling pick, ensuring the types align for
pick's arguments.

In `@apps/web/src/components/card-pull-request/timeline-pull-request.tsx`:
- Around line 132-134: The Text node rendering the coding duration uses
{timeToCode && <>{humanizeDuration(timeToCode)}</>} which omits the "in " prefix
and skips rendering when timeToCode is 0; update this to render consistently
with other entries by checking for a defined numeric value (e.g., timeToCode !==
null && timeToCode !== undefined or typeof timeToCode === "number") and render
the prefix string "in " followed by humanizeDuration(timeToCode) (referencing
the timeToCode variable and humanizeDuration function in the Text component) so
zero durations still display "in 0s" while maintaining consistency with other
timeline entries.

---

Nitpick comments:
In
`@apps/api/src/app/deployment/services/deployment-create-from-merge.service.ts`:
- Around line 56-80: The current sequence calls upsertDeployment,
linkPullRequestsToDeployment, and updatePullRequestDeploymentTracking separately
which can leave persisted state if the final call fails; wrap these three
operations in a single database transaction (or use the repository/ORM
transaction helper) so that upsertDeployment, linkPullRequestsToDeployment, and
updatePullRequestDeploymentTracking are executed atomically and rolled back on
error, or alternatively implement a compensating rollback when
updatePullRequestDeploymentTracking fails; locate these calls (upsertDeployment,
linkPullRequestsToDeployment, updatePullRequestDeploymentTracking) in
deployment-create-from-merge.service.ts and move them into a transactional block
that commits only after all succeed.

In `@apps/api/src/app/github/services/github-pull-request-tracking.service.ts`:
- Around line 99-104: The getTimeToDeploy function can return negative values if
deployedAt is earlier than mergedAt; update getTimeToDeploy (which calls
differenceInBusinessMilliseconds) to guard against negative results by ensuring
you compute max(0, differenceInBusinessMilliseconds(mergedAt, deployedAt)) or by
swapping/validating the dates before calling differenceInBusinessMilliseconds so
the function always returns zero or positive milliseconds.

In `@apps/api/src/app/pull-requests/services/pull-request.service.ts`:
- Around line 123-137: The function findPullRequestById always types its result
as PullRequest | null which ignores the shape of the include parameter; change
it to a generic signature so the include shape is propagated to the return type
(use Prisma's helper types). For example, make findPullRequestById generic (e.g.
<T extends Prisma.PullRequestInclude | null = null>), accept include?: T in
FindPullRequestByIdArgs, and set the return type to
Promise<Prisma.PullRequestGetPayload<{ include: T }> | null>; keep the existing
implementation body (getPrisma(...).pullRequest.findUnique(...)) but ensure the
include variable is typed as T so callers receive type-safe included relations
like tracking.

In `@apps/api/src/lib/logger.ts`:
- Line 37: The call to pinoLogger.info uses a redundant fallback
(pinoLogger.info(cleanObj || {}, msg)) even though cleanObj is already
initialized as obj || {} earlier; remove the dead guard and pass cleanObj
directly (i.e., change to pinoLogger.info(cleanObj, msg)) so the log call relies
on the guaranteed non-nullish cleanObj variable in logger.ts.

In `@packages/graphql-types/frontend/graphql.ts`:
- Line 1797: PullRequestsQuery includes the tracking.timeToCode field while
other PR-tracking queries (DeploymentQuery, TeamPullRequestsInProgressQuery,
TeamWorkLogQuery) do not; decide whether timeToCode should be consistently
returned and, if so, add timeToCode to the tracking selection in those queries
(or if intentional, leave PullRequestsQuery as-is and add a comment near
PullRequestsQuery explaining why timeToCode is only selected there). Update the
GraphQL query definitions for DeploymentQuery, TeamPullRequestsInProgressQuery,
and TeamWorkLogQuery (or add the explanatory comment in PullRequestsQuery) to
reflect the chosen consistency.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/lib/logger.ts">

<violation number="1" location="apps/api/src/lib/logger.ts:24">
P2: Incorrect field names for `pullRequest` in loggable fields. The PullRequest model uses `title` (not `name`) and doesn't have a `description` field. This will silently drop the PR title from every info log since these fields are undefined on the model.</violation>

<violation number="2" location="apps/api/src/lib/logger.ts:29">
P2: Avoid mutating the caller’s log object; clone it before sanitizing so logging doesn’t change application state.</violation>
</file>

<file name="apps/api/src/app/deployment/services/deployment-pr-linking.service.ts">

<violation number="1" location="apps/api/src/app/deployment/services/deployment-pr-linking.service.ts:174">
P3: Typo in error message: "Depoloyed" should be "Deployed".</violation>

<violation number="2" location="apps/api/src/app/deployment/services/deployment-pr-linking.service.ts:200">
P2: Guard for missing pullRequest.tracking (or use a safe update) before calling `pullRequestTracking.update`, otherwise PRs without tracking rows will throw and stop deployment linking.</violation>
</file>

<file name="apps/api/src/app/github/services/github-pull-request.service.ts">

<violation number="1" location="apps/api/src/app/github/services/github-pull-request.service.ts:204">
P2: Missing error handling on REST API call. If this GitHub API call fails (rate limit, 404 for deleted repo, network error), the entire `upsertPullRequest` flow will throw and the PR sync will fail. Wrap in try/catch and return `undefined` on error, similar to `getFirstCommit`.</violation>
</file>

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@apps/api/src/lib/logger.ts`:
- Line 42: The Sentry breadcrumb is currently being sent the unsanitized
original object (data: obj) and thus bypasses the redaction done earlier with
pick/cleanObj; update the breadcrumb to use the sanitized/derived value (e.g.,
data: cleanObj or JSON.stringify(cleanObj) if serialization is required) and
ensure you do not mutate obj—refer to the existing cleanObj and pick logic and
replace the usage of obj in the breadcrumb creation (the data: field) with the
sanitized cleanObj.

---

Duplicate comments:
In `@apps/api/src/app/deployment/services/deployment-pr-linking.service.ts`:
- Around line 200-212: The update call to
getPrisma(workspaceId).pullRequestTracking.update can throw Prisma P2025 if
pullRequest.tracking is null; replace the update with an upsert on
pullRequestTracking (use pullRequestId and workspaceId in the where clause, set
the same fields in data, and provide a create object with pullRequestId,
workspaceId, firstDeployedAt and timeToDeploy) so missing tracking rows are
created atomically and the Promise.all won't fail due to a missing row; locate
the call in deployment-pr-linking.service.ts (the block referencing
pullRequest.tracking and getTimeToDeploy) and swap update -> upsert and populate
create and update payloads accordingly.

In `@apps/api/src/lib/logger.ts`:
- Around line 31-34: The loop indexing causes TypeScript errors because cleanObj
is typed as {} and loggableFields is a narrow union; update the loop to use a
runtime guard "if (key in loggableFields)" and narrow/cast types before
indexing: treat cleanObj as Record<string, unknown> (or cast cleanObj[key] as
any/unknown) and cast loggableFields[key] to the appropriate type when calling
pick so TypeScript accepts cleanObj[key] and loggableFields[key]; adjust the
for-loop that iterates Object.keys(cleanObj) and references cleanObj,
loggableFields, and pick accordingly.

---

Nitpick comments:
In `@apps/api/src/lib/logger.ts`:
- Line 37: The expression uses a dead fallback: remove the redundant "|| {}" and
pass the actual cleanObj directly to pinoLogger.info (update the call site
pinoLogger.info(cleanObj || {}, msg) to pinoLogger.info(cleanObj, msg)); if the
original intent was to allow undefined, instead make cleanObj possibly undefined
at its initializer and keep the single-argument call consistent. Target the
pinoLogger.info call and the cleanObj variable in this file.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts">

<violation number="1" location="apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts:76">
P2: timeToDeploy should be null until the PR is merged; falling back to createdAt computes a misleading duration for unmerged PRs. Use mergedAt as the start time so calculateTimeForEvent returns null when it hasn’t merged yet.</violation>
</file>

<file name="apps/web/src/components/card-pull-request/use-badges.tsx">

<violation number="1" location="apps/web/src/components/card-pull-request/use-badges.tsx:183">
P2: The early return requires `isDeployed` to be true, which makes the “Not deployed”/“Stuck on deploy” label logic unreachable. This prevents the deploy badge from showing warning/error states for merged-but-not-deployed PRs.</violation>
</file>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/api/src/app/deployment/workers/deployment-triggered-by-pull-request-merge.worker.ts`:
- Around line 27-41: The code currently checks workspaceId but not
pullRequestId, so add a defensive guard for pullRequestId (from job.data) before
calling findPullRequestById: if pullRequestId is missing or falsy, throw a
ResourceNotFoundException("Pull request not found", { extra: { data: job.data }
}) to mirror the workspaceId check and avoid passing an undefined pullRequestId
into findPullRequestById; update any surrounding comments and ensure the thrown
exception type and extra payload match the existing workspaceId guard.

In
`@apps/api/src/app/pull-requests/resolvers/queries/pull-requests-tracking.query.ts`:
- Around line 26-33: transformPullRequestTracking is being fed Invalid Date
because parseISO is called on Prisma Date objects; change the construction of
the pullRequest object so you only call parseISO when the value is a string and
otherwise pass the Date through (or convert to ISO string if
transformPullRequestTracking expects strings). Concretely, update the fields
createdAt and mergedAt in the object passed to transformPullRequestTracking to:
if typeof pullRequest.createdAt === 'string' then
parseISO(pullRequest.createdAt) else pullRequest.createdAt (and same conditional
for mergedAt), so parseISO is never given a Date instance.

In `@apps/web/src/components/card-pull-request/use-badges.tsx`:
- Line 183: getLabel() currently has unreachable branches because the guard if
(!isNumber(timeToDeploy) || !isDeployed) return null forces isDeployed===true
for the rest of the function, so the "Stuck on deploy" and "Not deployed"
branches can never run; fix by either (A) simplifying getLabel() to only return
the "Deployed" label and remove the dead "Stuck on deploy"/"Not deployed"
branches if undeployed states are out of scope, or (B) support undeployed states
by changing the guard and/or inputs: stop gating on isDeployed with
timeToDeploy, introduce or use a separate metric (e.g., timeSinceMerge or
mergeTimestamp) and branch on isDeployed separately inside getLabel() so you can
return "Not deployed" (when !isDeployed and threshold not exceeded) or "Stuck on
deploy" (when !isDeployed and elapsed > threshold), updating any callers that
rely on timeToDeploy/isDeployed accordingly.

---

Outside diff comments:
In
`@apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts`:
- Around line 40-87: The timeToDeploy currently falls back to
pullRequest.createdAt when mergedAt is null, inflating deploy time for open PRs;
change the timeToDeploy field so it only calls calculateTimeForEvent when
pullRequest.mergedAt is present (e.g., timeToDeploy: pullRequest.mergedAt ?
calculateTimeForEvent({ from: pullRequest.mergedAt, duration:
tracking.timeToDeploy, pullRequest, uselessAfterMerge: false }) : null) so
open/unmerged PRs return null/absent deploy timing instead of a misleading
duration.

---

Duplicate comments:
In `@apps/web/src/components/card-pull-request/timeline-pull-request.tsx`:
- Around line 126-152: The "Coding" Timeline.Item currently renders just the
duration value (using timeToCode and humanizeDuration) but needs the same "in
{duration}" prefix used elsewhere; update the JSX inside Timeline.Item (where
timeToCode and humanizeDuration are used) to render the literal "in " before the
duration (e.g., output "in " + humanizeDuration(timeToCode) only when
isNumber(timeToCode) is true) so the display matches other steps.

---

Nitpick comments:
In `@apps/api/src/app/deployment/services/deployment-pr-linking.service.ts`:
- Around line 145-152: The current defensive check in
handleDeploymentPullRequestAutoLinking that throws DataIntegrityException when
filteredPullRequests contains a PR with a falsy mergedAt should be changed to
log a warning and early return instead of throwing; locate the
filteredPullRequests usage and the call site tied to
findMergedPullRequestsByCommitHashes (which already filters mergedAt: { not:
null }) and replace the throw with a processLogger.warn or equivalent warning
that includes the filteredPullRequests and the fact that mergedAt was
unexpectedly missing, then return early from
handleDeploymentPullRequestAutoLinking to avoid aborting the whole linking flow.
- Around line 160-177: The Promise.allSettled call over filteredPullRequests
currently discards settlement results so thrown DataIntegrityException and any
errors from updatePullRequestDeploymentTracking are silently ignored; change
this by either replacing Promise.allSettled with Promise.all to let rejections
propagate (so the caller sees failures) or keep allSettled but capture its
result array, iterate over the settlements and log or rethrow rejected entries
(including the reason and the associated pr) before returning; update the block
referencing Promise.allSettled, filteredPullRequests,
updatePullRequestDeploymentTracking, and DataIntegrityException accordingly to
ensure errors are surfaced or logged.

In
`@apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts`:
- Around line 24-37: When tracking is falsy in
pull-request-tracking.transformer.ts, the fallback object currently omits some
new fields which can lead to undefined values leaking to clients; update the
early return where it checks if (!tracking) to include explicit nulls for every
new nullable field (e.g., firstApprovalAt, firstReviewAt, timeToCode,
timeToFirstApproval, timeToFirstReview, timeToMerge, cycleTime) while keeping
existing defaults (size: PullRequestSize.MEDIUM, changedFilesCount: 0,
linesAddedCount: 0, linesDeletedCount: 0) so the output shape from the
transformer is stable and all fields are defined.

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@apps/api/src/app/deployment/workers/deployment-triggered-by-pull-request-merge.worker.ts`:
- Around line 13-53: No changes required: the previous issues are resolved by
adding the pullRequestId guard in deploymentTriggeredByPullRequestMergeWorker
and switching to fetching the PullRequest via findPullRequestById (with include:
{ tracking: true }) instead of serializing the full PullRequest in job data;
keep the current guards and call site type assertion as-is.

---

Nitpick comments:
In
`@apps/api/src/app/deployment/workers/deployment-triggered-by-pull-request-merge.worker.ts`:
- Around line 49-53: Split the combined null-check into two explicit checks so
the code distinguishes "pull request not found" from "tracking not found": first
check if pullRequest is falsy and throw ResourceNotFoundException with message
"Pull request not found" and include extra: { data: job.data, pullRequest } (or
omit pullRequest if null), then separately check if pullRequest.tracking is
falsy and throw ResourceNotFoundException with message "Pull request tracking
not found" including extra: { data: job.data, pullRequestId: pullRequest?.id,
tracking: pullRequest?.tracking } to aid debugging; update any surrounding uses
(e.g., in the worker handler where pullRequest is read) to assume tracking
exists only after the second check.

In `@packages/graphql-types/frontend/gql.ts`:
- Line 37: The generated queries duplicate the pull-request tracking field set
(timeToCode, timeToDeploy, firstDeployedAt, cycleTime and other tracking
subfields) across TeamPullRequestsInProgress and TeamWorkLog; update the source
.graphql files or the inline graphql() calls that define
TeamPullRequestsInProgress and TeamWorkLog to extract that repeated selection
into a shared GraphQL fragment (e.g., PullRequestTrackingFragment) and reference
that fragment from each query so the generated gql.ts (types.DeploymentDocument
/ the TeamPullRequestsInProgress and TeamWorkLog query documents) no longer
include verbatim duplicates.

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/api/src/app/deployment/services/deployment-pr-linking.service.ts`:
- Around line 161-182: Promise.allSettled on filteredPullRequests currently
discards results so rejections from updatePullRequestDeploymentTracking are
swallowed; change it to collect the settled results, iterate over them, and call
captureException for any result.status === "rejected" (include the rejection
reason and contextual info such as the associated pr, deployment, and
workspaceId). Locate the Promise.allSettled call around
filteredPullRequests.map(...) and the updatePullRequestDeploymentTracking
invoke, then after awaiting Promise.allSettled inspect each settled entry and
captureException with the error (and optionally wrap in a
DataIntegrityException-like message) for any rejected promise so errors are
logged instead of ignored.

In
`@apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts`:
- Around line 77-82: The timeToDeploy field is incorrectly using
pullRequest.mergedAt || pullRequest.createdAt, causing open PRs to yield their
age; change the call in the timeToDeploy assignment to pass only
pullRequest.mergedAt (remove the "|| pullRequest.createdAt" fallback) so
calculateTimeForEvent can hit its existing !from guard and return null for
un-merged PRs; update the timeToDeploy assignment where calculateTimeForEvent is
invoked to use from: pullRequest.mergedAt and keep uselessAfterMerge: false.
- Around line 56-64: The live calculation for timeToFirstApproval is using
tracking.firstApprovalAt as the earliest "from" candidate which yields a
negative/incorrect elapsed time when duration is null; change the call to
calculateTimeForEvent inside timeToFirstApproval to remove
tracking.firstApprovalAt from the from chain so it uses tracking.firstReadyAt ||
pullRequest.createdAt as the baseline (leave tracking.timeToFirstApproval as the
duration check intact because the precomputed-duration path already covers the
approved case).

---

Nitpick comments:
In
`@apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts`:
- Around line 92-101: Change the calculateTimeForEvent function signature to
require the pullRequest parameter (make pullRequest: PullRequest instead of
pullRequest?: PullRequest), update the internal checks that use optional
chaining (replace pullRequest?.state with pullRequest.state) and ensure the
existing CLOSED/uselessAfterMerge guard logic still runs; this prevents callers
from omitting pullRequest and lets you remove the unnecessary null-safe accesses
while preserving the current behavior around pullRequest.state and
uselessAfterMerge.

In
`@apps/web/src/app/metrics-and-insights/components/card-dora-metric/dora-card-stat.tsx`:
- Around line 106-111: The ternary rendering for
IconArrowUpRight/IconArrowDownRight can be simplified: since the surrounding
code already ensures change !== 0, replace the condition ((change >= 0 &&
higherIsBetter) || (change < 0 && !higherIsBetter)) with the simpler expression
(change > 0) === higherIsBetter so the arrow direction logic (used with
IconArrowUpRight and IconArrowDownRight) is clearer and remains consistent with
getTrendColor().

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/web/src/app/metrics-and-insights/page.tsx">

<violation number="1" location="apps/web/src/app/metrics-and-insights/page.tsx:133">
P2: The fallback for previousAmount never triggers because the string concatenation happens before the `||`, leading to "undefined deployments" when previousAmount is missing. Apply the fallback before concatenating the suffix.</violation>
</file>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/api/src/app/github/services/github-pull-request.service.ts`:
- Line 212: The Octokit field response.data.merge_commit_sha is string | null,
which can leak null into the local mergeCommitSha (typed string | undefined);
change the code to normalize null to undefined (e.g. assign
response.data.merge_commit_sha ?? undefined or use a conditional ternary) before
storing into mergeCommitSha or returning it so the value becomes string |
undefined; update the return expression that currently returns
response.data.merge_commit_sha to return response.data.merge_commit_sha ??
undefined instead (or explicitly map null -> undefined wherever mergeCommitSha
is assigned).

In
`@apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts`:
- Around line 56-64: The live calculation for timeToFirstApproval currently uses
tracking.firstReviewAt as the primary "from" baseline which undercounts waiting
time; change the baseline ordering in the calculateTimeForEvent call so it
prefers the PR entering the review queue (tracking.firstReadyAt) first, then
falls back to pullRequest.createdAt (and only use tracking.firstReviewAt as a
last-resort fallback if needed), keeping the existing duration
(tracking.timeToFirstApproval) and options (e.g., uselessAfterMerge: true)
intact; update the from expression in the timeToFirstApproval invocation (the
calculateTimeForEvent call) accordingly.

---

Outside diff comments:
In `@apps/web/src/app/metrics-and-insights/page.tsx`:
- Around line 171-186: The MTTR fallback strings are inconsistent with
getAbbreviatedDuration output; replace the literal "0 hours" used in the
CardDoraMetric props for amount and previousAmount with a call to
getAbbreviatedDuration(0) (or the function's zero-value equivalent) so
metrics.meanTimeToRecover?.currentAmount ? getAbbreviatedDuration(...) :
getAbbreviatedDuration(0) and likewise for previousAmount, referencing
CardDoraMetric and metrics.meanTimeToRecover to locate the code.

---

Duplicate comments:
In `@apps/api/src/app/deployment/services/deployment-pr-linking.service.ts`:
- Around line 161-182: Replace the Promise.all on filteredPullRequests with
Promise.allSettled and handle each settled result individually: call
updatePullRequestDeploymentTracking (same args: pullRequest pr as PullRequest &
{ tracking: PullRequestTracking }, deployment, workspaceId) for every PR, then
iterate the allSettled results and on any rejected result or returned
DataIntegrityException-related failure call captureException with context
(include pr and deployment/workspaceId) and do not let one failure short-circuit
others; ensure the function still awaits completion of all per-PR operations
before returning.

In
`@apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts`:
- Around line 77-82: Remove the fallback to pullRequest.createdAt in the
timeToDeploy call so that calculateTimeForEvent receives from:
pullRequest.mergedAt only; specifically update the timeToDeploy construction
(where calculateTimeForEvent is called) to pass from: pullRequest.mergedAt (not
pullRequest.mergedAt || pullRequest.createdAt) so the existing !from guard
inside calculateTimeForEvent will return null for unmerged/open PRs and avoid
computing the PR's age, keeping uselessAfterMerge: false as-is.

In `@apps/api/src/lib/logger.ts`:
- Around line 29-38: cleanObj and loggableFields cause TS index errors because
cleanObj is inferred as {} and loggableFields isn't indexed by string; update
types and add a runtime/type guard: declare cleanObj as Record<string, unknown>
(or cast obj to Record<string, unknown>) so cleanObj[key] is valid, and change
loggableFields' type to Record<string, string[]> (or assert key is keyof typeof
loggableFields) and add a guard like "if (key in loggableFields)" before
indexing; keep the existing typeof/nonnull check and continue to call
pick(cleanObj[key], loggableFields[key]) once the guards and types are fixed.
- Line 46: The Sentry breadcrumb currently gets the original unsanitized obj
(data: obj) which leaks fields excluded by loggableFields; replace that with the
sanitized object produced earlier (e.g., use the sanitized/filtered variable
returned by your sanitize/filter function) so the breadcrumb's data uses only
allowed fields, updating the code paths in the logger function where obj is
passed to Sentry (look for the breadcrumb or Sentry.addBreadcrumb call and the
obj / sanitized variable names) to pass the sanitized payload instead of obj.

---

Nitpick comments:
In
`@apps/api/src/app/pull-requests/resolvers/transformers/pull-request-tracking.transformer.ts`:
- Around line 97-101: The parameter object in the transformer that currently
types pullRequest as optional (the destructured param with from: Date | null;
duration: bigint | null; pullRequest?: PullRequest; uselessAfterMerge?: boolean)
should be changed so pullRequest is required (remove the optional marker) to
reflect that all call sites always supply it; update the type in the function
signature in pull-request-tracking.transformer.ts (and any exported
types/interfaces used there) to make pullRequest: PullRequest, then run
TypeScript to fix any leftover signatures — callers shouldn't need changes but
this enforces the invariant and prevents future omissions.
- Around line 42-45: The ternary that computes startedCodingAt is written with a
reversed sense (tracking.firstCommitAt && tracking.firstCommitAt >
pullRequest.createdAt ? pullRequest.createdAt : (tracking.firstCommitAt ??
pullRequest.createdAt)) which is correct but hard to read; change it to
explicitly take the earlier of tracking.firstCommitAt and pullRequest.createdAt
(e.g., add a small helper like getEarlierDate(a,b) or use a clear min-date
expression) and then set startedCodingAt using that helper/clear expression so
the intent is obvious; update references to startedCodingAt,
tracking.firstCommitAt and pullRequest.createdAt in
pull-request-tracking.transformer.ts accordingly.

In `@apps/api/src/lib/logger.ts`:
- Around line 49-93: The other logger methods (warn, error, debug, trace, fatal)
pass raw obj through while only info applies field sanitization; extract the
sanitization loop into a shared helper (e.g., sanitizeObj or cleanObj) and call
it at the top of each method (warn, error, debug, trace, fatal) so you pass
sanitizedObj to pinoLogger.* and to addBreadcrumb, and also correct the trace
breadcrumb level to "trace" instead of "debug"; update references to the
existing pinoLogger.warn/error/debug/trace/fatal and addBreadcrumb calls to use
the sanitized object.